Conversation
There was a problem hiding this comment.
은진님 고생 많으셨습니다!
이번주 핵심적인 컴포넌트 분리랑 state 위치 잡는 방향을 전반적으로 잘 잡으셨어요. 특히 todos를 App에 두고 입력값은 TodoInput에서 관리한 점은 이번 주 핵심을 잘 반영한 구현이에요.
이번주 코멘트는 일부 수정하면 좋을 부분들을 남겨두었습니다
질문에 대한 답변입니다
어떤 함수가 어느 위치에 있어야 하는지는 “어떤 state를 누가 소유하고 있느냐”를 기준으로 보면 훨씬 정리하기 쉬워요.
보통 shared state는 그 state를 소유한 컴포넌트(또는 가장 가까운 공통 부모)에 두고, 해당 state를 갱신하는 handler도 그 근처에 둡니다. 자식 컴포넌트는 props로 전달받아 실행만 하는 경우가 많아요.
이번 코드에서는 todos를 App이 관리하고 있으니 addTodo, deleteTodo 같은 함수도 App에 두는 구조가 가장 자연스럽습니다.
반면 문자열 가공, 날짜 포맷팅, 공통 계산처럼 UI 상태와 직접 연결되지 않은 순수 함수들은 lib/utils.ts 같은 별도 파일로 분리하는 편이 좋습니다 :)
| <del>{text}</del> | ||
| </div> | ||
| {name} | ||
| <button className="delBtn">🗑</button> |
There was a problem hiding this comment.
완료된 항목 쪽 삭제 버튼에는 onClick이 연결되어 있지 않아서, 지금은 완료된 todo를 삭제할 수 없어요. 완료 여부와 관계없이 같은 삭제 핸들러가 연결되도록 맞춰 주세요
| function handleFocus() { | ||
| if (text === "") { | ||
| setText("새로운 할 일"); | ||
| } |
There was a problem hiding this comment.
포커스할 때 새로운 할 일을 실제 input state로 넣으면, 사용자가 아무 것도 입력하지 않아도 그 문구가 그대로 추가될 수 있어요. 이 문구는 placeholder처럼만 보이게 하면 더 좋을 것같아요.
| function deleteTodo(id: number) { | ||
| const result = []; | ||
|
|
||
| for (let i = 0; i < todos.length; i++) { | ||
| if (todos[i].id !== id) { | ||
| result.push(todos[i]); | ||
| } | ||
| } | ||
|
|
||
| setTodos(result); | ||
| } |
There was a problem hiding this comment.
여기서는 직접 for문으로 새 배열을 만드는 것보다 todos.filter((todo) => todo.id !== id)처럼 쓰면, '이 id만 제외한다'는 의도가 더 바로 보여서 읽기 쉬워져요
| @@ -76,6 +76,52 @@ body { | |||
| line-height: 21px; | |||
| } | |||
There was a problem hiding this comment.
현재 클래스 명이 바뀌었는데 아직까지 text 클래스만 사용중인걸로 보여요. 스타일이 실제로 잘 적용되는지 확인해보면 좋을 것 같아요.
| if (completed) { | ||
| return ( | ||
| <li className={`item checked`}> | ||
| <div className="CheckedBox"> | ||
| <svg | ||
| className="CheckedIcon" | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| width="14" | ||
| height="10" | ||
| viewBox="0 0 14 10" | ||
| fill="none" | ||
| > | ||
| <g clip-path="url(#clip0_16_101)"> | ||
| <path | ||
| d="M1 5L5 9L13 1" | ||
| stroke="white" | ||
| stroke-width="2" | ||
| stroke-linecap="round" | ||
| stroke-linejoin="round" | ||
| /> | ||
| </g> | ||
| <defs> | ||
| <clipPath id="clip0_16_101"> | ||
| <rect width="14" height="10" fill="white" /> | ||
| </clipPath> | ||
| </defs> | ||
| </svg> | ||
| <img src={checkedIcon} alt="checked" /> | ||
| </div> | ||
| <div className="text"> | ||
| <del>{text}</del> | ||
| </div> | ||
| {name} | ||
| <button className="delBtn">🗑</button> | ||
| </li> | ||
| ); | ||
| } | ||
|
|
||
| return ( | ||
| <li className="item"> | ||
| <div className="UncheckedBox"></div> | ||
| {name} | ||
| <div className="text">{text}</div> | ||
| <button className="delBtn" onClick={handleClick}> | ||
| 🗑 | ||
| </button> | ||
| </li> | ||
| ); | ||
| } |
There was a problem hiding this comment.
| if (completed) { | |
| return ( | |
| <li className={`item checked`}> | |
| <div className="CheckedBox"> | |
| <svg | |
| className="CheckedIcon" | |
| xmlns="http://www.w3.org/2000/svg" | |
| width="14" | |
| height="10" | |
| viewBox="0 0 14 10" | |
| fill="none" | |
| > | |
| <g clip-path="url(#clip0_16_101)"> | |
| <path | |
| d="M1 5L5 9L13 1" | |
| stroke="white" | |
| stroke-width="2" | |
| stroke-linecap="round" | |
| stroke-linejoin="round" | |
| /> | |
| </g> | |
| <defs> | |
| <clipPath id="clip0_16_101"> | |
| <rect width="14" height="10" fill="white" /> | |
| </clipPath> | |
| </defs> | |
| </svg> | |
| <img src={checkedIcon} alt="checked" /> | |
| </div> | |
| <div className="text"> | |
| <del>{text}</del> | |
| </div> | |
| {name} | |
| <button className="delBtn">🗑</button> | |
| </li> | |
| ); | |
| } | |
| return ( | |
| <li className="item"> | |
| <div className="UncheckedBox"></div> | |
| {name} | |
| <div className="text">{text}</div> | |
| <button className="delBtn" onClick={handleClick}> | |
| 🗑 | |
| </button> | |
| </li> | |
| ); | |
| } | |
| return ( | |
| <li className={`item ${completed ? "checked" : ""}`}> | |
| <div className={completed ? "CheckedBox" : "UncheckedBox"}> | |
| {completed && <img src={checkedIcon} alt="" />} | |
| </div> | |
| <div className="text">{completed ? <del>{text}</del> : text}</div> | |
| <button className="delBtn" onClick={handleClick}> | |
| 🗑 | |
| </button> | |
| </li> | |
| ); | |
| } |
| function deleteTodo(id: number) { | ||
| const result = []; | ||
|
|
||
| for (let i = 0; i < todos.length; i++) { | ||
| if (todos[i].id !== id) { | ||
| result.push(todos[i]); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
addTodo, deleteTodo도 동작은 잘 전달되지만, 이벤트를 처리하는 함수라는 점이 더 잘 드러나도록 handleAddTodo, handleDeleteTodo처럼 이름을 지어보는 것도 좋아요. 그리고 자식 컴포넌트에 props로 넘길 때는 onDelete처럼 역할이 보이는 이름을 쓰면, 이건 콜백 props구나가 더 바로 읽힙니다.
2 participants
✅ 제출 정보
✅ 체크리스트 (필수)
main이 아니라 내 GitHub 핸들 브랜치(<handle>)다<handle>-week-xx형식이다🧩 구현 내용 요약
❓ 궁금한 점